-
Notifications
You must be signed in to change notification settings - Fork 993
Minor internal api cleanups to improve comments and avoid errors #1752
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…-throwing orderBy option
| serializeOrdersQueryOptions({ | ||
| limit: 1, | ||
| orderBy, | ||
| orderBy: "created_date", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
here is what the docs say, maybe we should just update the typedoc to include this?
|
|
@ryanio i can update the PR to do that! i think i understand better now what the endpoint is for, i realise that each token might have multiple listings (in my mind i was only thinking of the "active" ie cheapest listing and wanted an endpoint to get me the listings of a collection, sorted by eth price such that it gives the "sort by price" view on the collection page) |
|
yeah it's a bit confusing. we used to have multiple listing currencies in OS1 but now in OS2 every chain has one listing currency (usually native, except for polygon is WETH) and one offer currency (usually wrapped native, polygon is WETH). there are helper fns in opensea-js now to get listing currency or offer currency for a chain if that's helpful for you. |
Updates documentation for getOrder and getOrders to clarify that when using orderBy: "eth_price", the asset_contract_address and token_ids parameters must also be provided, as noted in the API documentation. Fixes the confusion raised in #1752 without removing the parameters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
) * Clarify JSDoc for orderBy eth_price requirement Updates documentation for getOrder and getOrders to clarify that when using orderBy: "eth_price", the asset_contract_address and token_ids parameters must also be provided, as noted in the API documentation. Fixes the confusion raised in #1752 without removing the parameters. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Add runtime validation for eth_price orderBy parameter - Throw descriptive error when orderBy is "eth_price" but required params are missing - Added validation to both getOrder and getOrders methods - Error message clearly indicates both asset_contract_address and token_ids are required - This provides early error detection before API call 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * test: remove chain exclusions from getCollections test - Remove Abstract, ApeChain, Blast, and Zora exclusions - Keep only Solana excluded (no NFT collections) - Test now runs for all chains except Solana 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
|
fixed in #1774, thanks! |

Motivation
For a personal project I needed to generate the calldata for (but not actually fulfill) a listed NFT. The SDK has an internal api.ts that supports this but from my brief searching it looks like it isn't exposed.
Solution
I ended up forking the repo and using it locally. In doing so, i found that the JSDoc for
api.getOrderandapi.getOrdershad anoptions.limitthat was in fact omitted in the typescript. Additionally, passing anorderBy: "eth_price"toapi.getOrderthrows an error, so I also removed that as a configurable option.A couple of other things i noticed: